Skip to content

Add outcome panels#1543

Merged
koesie10 merged 2 commits intomainfrom
koesie10/outcome-panel
Sep 27, 2022
Merged

Add outcome panels#1543
koesie10 merged 2 commits intomainfrom
koesie10/outcome-panel

Conversation

@koesie10
Copy link
Copy Markdown
Member

@koesie10 koesie10 commented Sep 26, 2022

This creates the component for showing the outcome panels. It does not implement the content of each individual panel; it only implements the tabs, panel views, and the general warnings.

Screenshot 2022-09-26 at 15 02 17

Screenshot 2022-09-26 at 15 03 26

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

This creates the component for showing the outcome panels. It does not
implement the content of each individual panel; it only implements the
tabs, panel views, and the general warnings.
Base automatically changed from koesie10/alert-components to main September 26, 2022 13:00
@koesie10 koesie10 marked this pull request as ready for review September 26, 2022 13:00
@koesie10 koesie10 requested review from a team as code owners September 26, 2022 13:00
Copy link
Copy Markdown
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question but otherwise all looks good to me.

<Alert
type="warning"
title="Repository limit exceeded"
message={`The number of requested repositories exceeds the maximum number of repositories supported by multi-repository variant analysis. ${overLimitRepositoryCount} ${overLimitRepositoryCount === 1 ? 'repository was' : 'repositories were'} skipped.`}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm just missing them but I can't see this warning or the "access mismatch" warning as part of the designs. Do we need general warning above the tabs, or is it enough just to have the tabs and then there are warnings inside that explain why repos were skipped?

We can leave the implementation of the warnings inside the tabs for when implementing those tabs.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We talked about this and I was getting confused between the different error states. These general warnings are for the "too many repos" and "public controller repo" cases, which are different from the "no access" and "no db" cases which get their own tabs. So these warnings are all good.

Regarding the specific text for them, let's go with this for now as just a placeholder while everything is private. We'll have a discussion offline about what text to use here and in general throughout the extension.

@robertbrignull
Copy link
Copy Markdown
Contributor

Also need to fix up some merge conflicts from #1541

Copy link
Copy Markdown
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@koesie10 koesie10 merged commit 110d930 into main Sep 27, 2022
@koesie10 koesie10 deleted the koesie10/outcome-panel branch September 27, 2022 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants